HTM-1438: Delete scheduled task by setting search index schedule to no schedule #1044
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the task deletion and scheduling functionality in the search index management system. The changes convert TaskMonitoringService to a root-level singleton service and improve the handling of schedule deletions.
Key changes:
- Converted
TaskMonitoringServicefrom component-level to root-level singleton service - Implemented automatic task deletion when removing a search index schedule
- Enabled the "No schedule" option in the UI that was previously disabled
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| projects/shared/src/lib/components/slider/slider.component.ts | Removed debug console statement |
| projects/admin-core/src/lib/tasks/task-details/task-details.component.ts | Removed service from component providers, fixed method name to follow naming convention |
| projects/admin-core/src/lib/tasks/services/task-monitoring.service.ts | Changed service to root-level singleton, renamed method to follow RxJS naming convention |
| projects/admin-core/src/lib/search-index/services/search-index.service.ts | Added logic to delete associated tasks when removing schedules, imported required dependencies |
| projects/admin-core/src/lib/search-index/search-index-scheduling/search-index-scheduling.component.ts | Simplified schedule object creation to always build schedule object |
| projects/admin-core/src/lib/search-index/search-index-scheduling/search-index-scheduling.component.html | Removed disabled state from "No schedule" option |
| projects/admin-core/src/lib/search-index/search-index-edit/search-index-edit.component.ts | Added task loading on component initialization |
Comments suppressed due to low confidence (1)
projects/admin-core/src/lib/tasks/services/task-monitoring.service.ts:78
- The method dispatches
deleteTaskSuccesswiththis.uuid$.valueinstead of theuuidparameter passed to the method. SinceTaskMonitoringServiceis now a root-level singleton used by multiple components,this.uuid$.valuemay contain a different task UUID than the one being deleted. Use theuuidparameter instead:deleteTaskSuccess({ taskUuid: uuid }).
this.store$.dispatch(deleteTaskSuccess({ taskUuid: this.uuid$.value }));
projects/admin-core/src/lib/search-index/services/search-index.service.ts
Show resolved
Hide resolved
projects/admin-core/src/lib/search-index/services/search-index.service.ts
Outdated
Show resolved
Hide resolved
| const schedule = { | ||
| ...this.taskSchedule, | ||
| cronExpression: value.cronExpression || '', | ||
| description: value.description, | ||
| priority: value.priority, | ||
| }; |
There was a problem hiding this comment.
The schedule object is now always created even when cronExpression is empty. This changes the behavior from emitting undefined to emitting a schedule object with an empty cronExpression. This could cause issues in the parent component logic that checks for schedule existence. Consider preserving the original behavior where schedule is set to undefined when cronExpression is empty.
| const schedule = { | |
| ...this.taskSchedule, | |
| cronExpression: value.cronExpression || '', | |
| description: value.description, | |
| priority: value.priority, | |
| }; | |
| let schedule: TaskSchedule | undefined; | |
| if (value.cronExpression && value.cronExpression.trim() !== '') { | |
| schedule = { | |
| ...this.taskSchedule, | |
| cronExpression: value.cronExpression, | |
| description: value.description, | |
| priority: value.priority, | |
| }; | |
| } else { | |
| schedule = undefined; | |
| } |
There was a problem hiding this comment.
@steinkobben this seems a valid concern, see also: #1044 (comment)
There was a problem hiding this comment.
I have added a check that prevents creating new schedules if the cron expression is empty. When there is an existing schedule, the schedule should be emitted with an empty cron expression because the parent component needs the schedule uuid to delete the task.
...admin-core/src/lib/search-index/search-index-scheduling/search-index-scheduling.component.ts
Outdated
Show resolved
Hide resolved
projects/admin-core/src/lib/search-index/services/search-index.service.ts
Outdated
Show resolved
Hide resolved
Uh oh!
There was an error while loading. Please reload this page.